Add json parse and stringify function to jerryscript c api #2243

ZsoltRaduska · 2018-03-19T10:52:07Z

No description provided.

martijnthe · 2018-03-19T11:07:09Z

jerry-core/include/jerryscript-core.h

 jerry_value_t jerry_get_typedarray_buffer (jerry_value_t value,
                                           jerry_length_t *byte_offset,
                                           jerry_length_t *byte_length);
+jerry_value_t jerry_json_parse (const char * json_string);


I think it would be more consistent with the rest of the API if this would take a jerry_value_t representing the string.

martijnthe · 2018-03-19T11:10:13Z

jerry-core/include/jerryscript-core.h

                                           jerry_length_t *byte_offset,
                                           jerry_length_t *byte_length);
+jerry_value_t jerry_json_parse (const char * json_string);
+jerry_value_t jerry_json_stringfy (const jerry_value_t object_to_stringify);


JSON.parse and JSON.stringify take a couple of optional arguments.
I guess they could be exposed as different C APIs (i.e. jerry_json_parse_with_reviver, etc)

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify

robertsipka · 2018-03-19T17:36:06Z

tests/unit-core/test-api.c


-  char string_console[cesu8_sz];
+  char string_console[7];
  jerry_string_to_char_buffer (args[0], (jerry_char_t *) string_console, cesu8_sz);


Please rewrite this to the original value (do not use magic constants).

robertsipka · 2018-03-19T17:37:51Z

docs/02.API-REFERENCE.md


 - [jerry_create_typedarray](#jerry_create_typedarray)
+
+


unnecessary new line

robertsipka · 2018-03-19T17:38:40Z

docs/02.API-REFERENCE.md

+  jerry_release_value (parsed_json);
+}
+```
+## jerry_stringify


add a new line here before the heading

robertsipka · 2018-03-19T17:40:53Z

docs/02.API-REFERENCE.md

+## jerry_stringify
+
+  **Summary**
+  Stringify a jerry_value_t object


Add a new line here after the emphasized text

robertsipka · 2018-03-19T17:42:24Z

docs/02.API-REFERENCE.md

+
+```c
+jerry_value_t jerry_json_parse (const char * json_string)
+```


remove space after the asterisk. (const char *json_string)

robertsipka · 2018-03-19T18:31:34Z

tests/unit-core/test-api.c

+    jerry_value_t has_prop_js = jerry_has_property (parsed_json, key);
+    bool has_prop = jerry_get_boolean_value (has_prop_js);
+    TEST_ASSERT (has_prop == true);
+    jerry_release_value (has_prop_js);


jerry_value_t has_key_prop = jerry_has_property (parsed_json, key);
TEST_ASSERT (jerry_get_boolean_value (has_key_prop));
jerry_release_value (has_key_prop);

robertsipka · 2018-03-19T18:32:38Z

tests/unit-core/test-api.c

+    bool has_prop = jerry_get_boolean_value (has_prop_js);
+    TEST_ASSERT (has_prop == true);
+    jerry_release_value (has_prop_js);
+    parsed_data = jerry_get_property (parsed_json, key);


You can declare parsed_data here also

robertsipka · 2018-03-19T18:33:03Z

tests/unit-core/test-api.c

+    char buff[jerry_get_string_length (parsed_data)];
+    jerry_char_t *buff_p = (jerry_char_t *) buff;
+    jerry_string_to_char_buffer (parsed_data, buff_p, buff_size);
+    buff[buff_size] = '\0';


Why this needed?

Without it there is an error in utittests, as I told you.

What about using strncmp instead of strcmp?

robertsipka · 2018-03-19T18:33:54Z

tests/unit-core/test-api.c

+    jerry_char_t *buff_p = (jerry_char_t *) buff;
+    jerry_string_to_char_buffer (parsed_data, buff_p, buff_size);
+    buff[buff_size] = '\0';
+    TEST_ASSERT (strcmp ((const char *) data_check , (const char *) buff)  == false);


I do not think it is necessary to create a variable for the data_check content, just put the string here.

robertsipka · 2018-03-19T18:34:35Z

tests/unit-core/test-api.c

+    jerry_string_to_char_buffer (stringified, (jerry_char_t *) buff,
+                                (jerry_size_t) jerry_get_string_length (stringified));
+    buff[jerry_get_string_length (stringified)] = '\0';
+    TEST_ASSERT (strcmp ((const char *) check_value , (const char *) buff)  == 0);


double spaces.

I do not think it is necessary to create a variable for the check_value content, just put the string here.

robertsipka · 2018-03-19T18:40:59Z

Sorry, I've just realized that this is a WIP PR.

robertsipka · 2018-03-22T14:46:31Z

jerry-core/ecma/builtin-objects/ecma-builtin-helpers-json.c

 * @}
 */
+
+#endif /* !CONFIG_DISABLE_JSON_BUILTIN */


It is outside from the comment group, please put it before the end of the group.

zherczeg · 2018-03-23T14:18:39Z

docs/02.API-REFERENCE.md

+**Summary**
+
+Parse a JSON string.
+Returns a jerry_value_t which contains key-value pairs imported from the JSON string.


Comment: Returns the same result as JSON.parse ecmascript function.

zherczeg · 2018-03-23T14:19:02Z

docs/02.API-REFERENCE.md

+  const char *data_in_json = "{\"name\": \"John\", \"age\": 5}";
+  jerry_value_t parsed_json = jerry_json_parse (data_in_json);
+
+  // parsed_json now conatins all of data stored in data_in_json


zherczeg · 2018-03-23T14:20:23Z

docs/02.API-REFERENCE.md

+  **Summary**
+
+  Stringify a jerry_value_t object.
+  Convert a jerry_value_t object into a json formated string.


Comment: Returns the same value as JSON.stringify() ecmascript function.

zherczeg · 2018-03-23T14:23:03Z

jerry-core/ecma/builtin-objects/ecma-builtin-helpers.h

+                                    bool parse_string); /**< strings are allowed to parse */
+ecma_object_t *ecma_op_create_object_object_noarg (void);
+ecma_value_t
+ecma_op_object_put (ecma_object_t *object_p, /**< the object */


I think these functions are exported elsewhere.

zherczeg · 2018-03-23T14:24:45Z

jerry-core/ecma/builtin-objects/ecma-builtin-json.c

 * @return ecma value
 *         Returned value must be freed with ecma_free_value.
 */
+


No need this newline.

zherczeg · 2018-03-23T14:26:11Z

jerry-core/ecma/builtin-objects/ecma-builtin-helpers.h

+  left_square_token, /**< JSON left square bracket */
+  comma_token, /**< JSON comma */
+  colon_token /**< JSON colon */
+} ecma_json_token_type_t;


These types should not be exported.

zherczeg · 2018-03-23T14:26:40Z

jerry-core/ecma/builtin-objects/ecma-builtin-helpers.h


+ecma_value_t ecma_builtin_json_parse_value (ecma_json_token_t *token_p);
+void ecma_builtin_json_parse_next_token (ecma_json_token_t *token_p, /**< token argument */
+                                    bool parse_string); /**< strings are allowed to parse */


Misaligned argument.

zherczeg

LGTM

LaszloLango · 2018-03-28T08:00:53Z

docs/02.API-REFERENCE.md

+
+- `string_p` - a JSON string
+- `string_size` - size of the string
+


missing return statement

LaszloLango · 2018-03-28T08:01:12Z

docs/02.API-REFERENCE.md

+```
+
+- `object_to_stringify` - a jerry_value_t object to stringify
+


missing return statement

LaszloLango · 2018-03-28T08:02:10Z

jerry-core/api/jerry.c

+ *
+ * Note:
+ *      The returned value must be freed with jerry_release_value
+ * @return jerry_value_t from json formated string


We should mention that the return value can be an error.

LaszloLango · 2018-03-28T08:02:29Z

jerry-core/api/jerry.c

+ *
+ * Note:
+ *      The returned value must be freed with jerry_release_value
+ * @return json formated jerry_value_t


LaszloLango · 2018-03-28T08:02:57Z

jerry-core/ecma/builtin-objects/ecma-builtin-helpers.h

 } ecma_json_stringify_context_t;

+ecma_value_t ecma_builtin_json_parse_buffer (const lit_utf8_byte_t * str_start_p,
+                                                              lit_utf8_size_t string_size);


wrong indentation

LaszloLango · 2018-03-28T08:03:18Z

jerry-core/ecma/builtin-objects/ecma-builtin-json.c

+ */
+ecma_value_t
+ecma_builtin_json_parse_buffer (const lit_utf8_byte_t * str_start_p, /**< String to parse */
+                                                lit_utf8_size_t string_size) /**< size of the string */


LaszloLango · 2018-03-28T08:04:05Z

jerry-core/ecma/builtin-objects/ecma-builtin-json.c

+/**
+ * Function to set a string token from the given arguments, fill its fields and advances the string pointer.
+ *
+ * @return ecma value


Please add more details for the return value.

LaszloLango · 2018-03-28T08:04:14Z

jerry-core/ecma/builtin-objects/ecma-builtin-json.c

+/**
+ * Helper functions to stringify an object in JSON format representing an ecma_value.
+ *
+ *  @return ecma value


LaszloLango · 2018-03-28T08:04:27Z

jerry-core/ecma/builtin-objects/ecma-builtin-json.c

+/**
+ * Function to create a json formated string from an object
+ *
+ * @return ecma value


LaszloLango · 2018-04-03T07:24:10Z

jerry-core/ecma/builtin-objects/ecma-builtin-json.c

 } /* ecma_builtin_json_walk */

+/**
+ * Function to set a string token from the given arguments, fill its fields and advances the string pointer.


LaszloLango · 2018-04-03T07:26:27Z

jerry-core/ecma/builtin-objects/ecma-builtin-json.c

 ecma_builtin_json_array (ecma_object_t *obj_p, ecma_json_stringify_context_t *context_p);

+/**
+ * Helper functions to stringify an object in JSON format representing an ecma_value.


LaszloLango · 2018-04-03T07:30:20Z

tests/unit-core/test-api.c

+    char buff[jerry_get_string_length (parsed_data)];
+    jerry_char_t *buff_p = (jerry_char_t *) buff;
+    jerry_string_to_char_buffer (parsed_data, buff_p, buff_size);
+    buff[buff_size] = '\0';


What about using strncmp instead of strcmp?

JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska rzsolt@inf.u-szeged.hu

LaszloLango

LGTM

* Add the ability to throw an error to python debugger (jerryscript-project#2188) This patch makes it possible to throw an error from the python debugger client using the `throw` command. JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu * Fix typo and redundant text in the documentation. (jerryscript-project#2260) JerryScript-DCO-1.0-Signed-off-by: Daniel Vince vinced@inf.u-szeged.hu * Fix accessing the contents of a direct string (jerryscript-project#2261) In the `ecma_string_get_chars` method the contents of a direct string is accessed incorrectly. It tries to extract the magic string id from the string pointer but the direct string does not need this step. JerryScript-DCO-1.0-Signed-off-by: Peter Gal pgal.u-szeged@partner.samsung.com * Remove websocket message macros in debugger (jerryscript-project#2262) JERRY_DEBUGGER_INIT_SEND_MESSAGE JERRY_DEBUGGER_SET_SEND_MESSAGE_SIZE JERRY_DEBUGGER_SET_SEND_MESSAGE_SIZE_FROM_TYPE JerryScript-DCO-1.0-Signed-off-by: Jimmy Huang jimmy.huang@intel.com * Remove TARGET_HOST macros (jerryscript-project#2264) Remove TARGET_HOST defines from the jerry-libc module and replace with compiler provided macros. JerryScript-DCO-1.0-Signed-off-by: Istvan Miklos imiklos2@inf.u-szeged.hu * Fix bug in stringToCesu8 conversion for 0x7ff (jerryscript-project#2267) JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson geoff@linux.intel.com * Add ecma_free_all_enqueued_jobs function (jerryscript-project#2265) This function releases any remaining promise job that wasn't completed. Also added this function to `jerry_cleanup ()`, therefore it will be automatically run. JerryScript-DCO-1.0-Signed-off-by: Daniel Balla dballa@inf.u-szeged.hu * Improve jerry_is_feature_enabled with object availability information (jerryscript-project#2250) JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska rzsolt@inf.u-szeged.hu * Add json parse and stringify function to jerryscript c api (jerryscript-project#2243) JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska rzsolt@inf.u-szeged.hu * Simplify debugger-ws.h (jerryscript-project#2266) Remove several macros and types from it. This change simplifies the external debugger interface. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com * Add finalize_cb to jerry_context_data_manager_t (jerryscript-project#2269) This patch adds a new finalize_cb callback to jerry_context_data_manager_t. The callback is run as the very last thing in jerry_cleanup, after the VM has been torn down entirely. There was already the deinit_cb, which is run while the VM is still in the process of being torn down. The reason the deinit_cb is not always sufficient is that there may still be objects alive (because they still being referenced) that have native pointers associated with the context manager that is being deinit'ed. As a result, the free_cb's for those objects can get called *after* the associated context manager's deinit_cb is run. This makes cleanup of manager state that is depended on by the live objects impossible to do in the deinit_cb. That type of cleanup can only be done when all values have been torn down completely. JerryScript-DCO-1.0-Signed-off-by: Martijn The martijn.the@intel.com * Rework snapshot generation API. (jerryscript-project#2259) Also remove eval context support. It provides no practical advantage. JerryScript-DCO-1.0-Signed-off-by: Zoltan Herczeg zherczeg.u-szeged@partner.samsung.com * Implement the ES2015 version of Object.getPrototypeOf and add a test file for it (jerryscript-project#2256) JerryScript-DCO-1.0-Signed-off-by: Peter Marki marpeter@inf.u-szeged.hu * Move DevTools integration code into jerry-client-ts subdir JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson geoff@linux.intel.com * Fix some things to match the new directory for TS code JerryScript-DCO-1.0-Signed-off-by: Geoff Gustafson geoff@linux.intel.com

…pt-project#2243) JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska rzsolt@inf.u-szeged.hu

martijnthe reviewed Mar 19, 2018

View reviewed changes

ZsoltRaduska force-pushed the extendCapi branch 2 times, most recently from 82fa30c to 68cb0b5 Compare March 19, 2018 14:59

robertsipka reviewed Mar 19, 2018

View reviewed changes

ZsoltRaduska force-pushed the extendCapi branch 6 times, most recently from 8f3af57 to e49ec89 Compare March 22, 2018 14:35

ZsoltRaduska changed the title ~~[WIP] Add json parse and stringify function to jerryscript c api~~ Add json parse and stringify function to jerryscript c api Mar 22, 2018

robertsipka reviewed Mar 22, 2018

View reviewed changes

ZsoltRaduska force-pushed the extendCapi branch 4 times, most recently from 1547e92 to 0c582d4 Compare March 23, 2018 07:58

zherczeg requested changes Mar 23, 2018

View reviewed changes

ZsoltRaduska force-pushed the extendCapi branch 2 times, most recently from fe73145 to 48403f3 Compare March 26, 2018 15:24

zherczeg mentioned this pull request Mar 27, 2018

Nominating Robert Sipka (robertsipka) to become a reviewer #2254

Closed

ZsoltRaduska force-pushed the extendCapi branch 8 times, most recently from b535c52 to a2c01aa Compare March 27, 2018 13:56

ZsoltRaduska force-pushed the extendCapi branch 3 times, most recently from b79de02 to 9b96e6b Compare March 27, 2018 15:11

zherczeg approved these changes Mar 28, 2018

View reviewed changes

LaszloLango requested changes Mar 28, 2018

View reviewed changes

ZsoltRaduska force-pushed the extendCapi branch 4 times, most recently from 125a86b to 8204979 Compare March 29, 2018 15:32

LaszloLango requested changes Apr 3, 2018

View reviewed changes

ZsoltRaduska force-pushed the extendCapi branch 3 times, most recently from c3a44e7 to 3043555 Compare April 3, 2018 22:05

Add json parse and stringify function to jerryscript c api

9da1df1

JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska rzsolt@inf.u-szeged.hu

ZsoltRaduska force-pushed the extendCapi branch from 3043555 to 9da1df1 Compare April 4, 2018 15:18

LaszloLango approved these changes Apr 5, 2018

View reviewed changes

LaszloLango merged commit 78bd11e into jerryscript-project:master Apr 5, 2018

jimmy-huang pushed a commit to jimmy-huang/jerryscript that referenced this pull request May 8, 2018

Add json parse and stringify function to jerryscript c api (jerryscri…

42bd6a7

…pt-project#2243) JerryScript-DCO-1.0-Signed-off-by: Zsolt Raduska rzsolt@inf.u-szeged.hu

akosthekiss mentioned this pull request Jan 12, 2019

JSON.parse/stringify C APIs? #1949

Closed


		- `string_p` - a JSON string
		- `string_size` - size of the string

		```

		- `object_to_stringify` - a jerry_value_t object to stringify

Add json parse and stringify function to jerryscript c api #2243

Add json parse and stringify function to jerryscript c api #2243

Uh oh!

Conversation

ZsoltRaduska commented Mar 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

robertsipka Mar 22, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zherczeg left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertsipka Mar 19, 2018 •

edited

Loading

robertsipka Mar 19, 2018 •

edited

Loading

robertsipka commented Mar 19, 2018 •

edited

Loading

robertsipka Mar 22, 2018 •

edited

Loading